drivers: eth_xmc4xxx: Add option to load MAC address from EEPROM #74835

talih0 · 2024-06-23T14:53:29Z

Adds an option to load the MAC address from an external EEPROM chip, for example from Microchip 25AA02E48 (atmel,at25) which come pre-programmed with a unique MAC.

The priority for setting the MAC address in the driver is: (1) local-mac-address from devicetree
(2) Eeprom address
(3) Random address

pdgendt

Why does an ethernet driver need to know where the mac address is coming from?

If you need this, you should implement/use the set_config from the ethernet driver API.

jukkar · 2024-06-24T07:30:52Z

If you need this, you should implement/use the set_config from the ethernet driver API.

I think the driver works ok with this respect, it implements the set_config which then sets the MAC address. This change is figuring out from where the mac address is generated from in driver init which is then setting the initial value.

pdgendt

This should be built in CI, maybe add a case for build_only tests?

pdgendt · 2024-06-24T07:37:56Z

drivers/ethernet/eth_xmc4xxx.c

Put the compile check around the function.

#if DT_INST_NODE_HAS_PROP(0, mac_eeprom) static int eth_xmc4xxx_get_mac_address_eeprom(const struct device *dev) { struct eth_xmc4xxx_data *dev_data = dev->data; const struct device *eeprom_dev = DEVICE_DT_GET(DT_INST_PHANDLE(0, mac_eeprom)); int ret; if (!device_is_ready(eeprom_dev)) { LOG_ERR("EEPROM device not ready."); return -ENODEV; } ret = eeprom_read(eeprom_dev, CONFIG_ETH_XMC4XXX_MAC_EEPROM_ADDRESS, dev_data->mac_addr, sizeof(dev_data->mac_addr)); if (ret < 0) { LOG_ERR("Error reading MAC address from EEPROM [%d]", ret); } return ret; } #endif

In my patch returning -ENODEV makes using the function cleaner and the compiler optimizes out the function when the node is not in the devicetree.

pdgendt · 2024-06-24T07:41:07Z

drivers/ethernet/Kconfig.xmc4xxx

We shouldn't be mixing Kconfig with device tree properties, it should be possible to have this added in the device tree:

mac-eeprom = <&eeprom 0x00>;

@pdgendt that would mean I'd have to modify the binding of the eeprom to add the -cells property?
@henrikbrixandersen would this be acceptable?

If we are to do this (and not just rely on an application loading the board-specific MAC address from wherever and configuring the network interface), I'd prefer we align with how the Linux kernel devicetree bindings do this.

Something along the lines of:

https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml

https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-e3c246d4i.dts (example use)

@pdgendt that would mean I'd have to modify the binding of the eeprom to add the -cells property?

I don't think it needs that see the docs

@pdgendt, unless I'm missing something, it is the binding of the referenced node that describes how the specifier entries are parsed. (i.e. all the gpio and dma examples).

https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/devicetree/gpio.h#L69
From this example it's the gpio binding that needs to define gpio-cells.

Correct. Please see the section on specifier spaces further down that page.

I'd still prefer using the same specifier space as the Linux kernel nvmem consumer bindings.

@henrikbrixandersen, I'll work on the nvmem consumer binding.

@henrikbrixandersen this an example from linux that matches what I'm trying to do:

But I have a devicetree question, what would be the default compatible of mac_address in this case? I tried the same approach in zephyr. It works, but it doesn't say which compatible was used for this node:

github-actions · 2024-08-25T00:32:19Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

talih0 · 2024-08-28T20:16:46Z

When testing on the sam_v71_xult board I found that the ethernet did not work independent of my change. The read MAC address from the EEPROM chip is the same before and after the change in this PR.

Adds property nvmem-cells for referencing a node that stores some configuration information. A typical use case is the reading of MAC address from an EEPROM device. The referenced node is child of the non-volatile memory device. The address of the referenced node stores address/offset where the configuration is stored. Th nvmem-cells phandle could be pointing to a child of an eeprom node or an efuse register. Currently it is upto the devicetree driver to call the correct API for reading the configuration data. Future work could be to write an nvmem-producer driver abstraction that calls correct api for flash/eeprom. Signed-off-by: Andriy Gelman <[email protected]>

Adds an option to load the MAC address from an external EEPROM chip, for example from Microchip 25AA02E48 (atmel,at25) which come pre-programmed with a unique MAC. The priority for setting the MAC address in the driver is: (1) local-mac-address from devicetree (2) Eeprom address (3) Random address Signed-off-by: Andriy Gelman <[email protected]>

nandojve

Tested on sam_v71_xult/samv71q21 samples/net/cloud/tagoio_http_post.

Outdated review

talih0 · 2025-02-04T12:06:12Z

@henrikbrixandersen, could you have another look, please?

github-actions · 2025-04-06T00:39:31Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

github-actions · 2025-06-06T00:38:55Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

talih0 · 2025-06-09T06:37:07Z

@henrikbrixandersen, could you have another look when you have a moment please

henrikbrixandersen · 2025-06-22T15:57:43Z

drivers/ethernet/Kconfig.sam_gmac

-	  Size (in bytes) of the internal EEPROM address.
-
-endif # ETH_SAM_GMAC_MAC_I2C_EEPROM
+	  Enable reading MAC address from external flash memory.


Suggested change

Enable reading MAC address from external flash memory.

Enable reading MAC address from external EEPROM memory.

henrikbrixandersen

Sorry about the long delay in reviewing, somehow this fell off my radar.

It's looking really, really good. I'd like to try getting this into Zephyr v4.2, although we're a bit pressed on time for this.

The tests for the macros can come in after the feature freeze, if needed.

henrikbrixandersen · 2025-06-22T15:59:51Z

drivers/ethernet/eth_sam_gmac.c

-
-	if (ret != 0) {
-		LOG_ERR("I2C: failed to read MAC addr");
+	ret = eeprom_read(eeprom_dev, addr, mac_addr, 6);


Shouldn't this at least check if the length matches that of the reg property in the devicetree instead of hardcoding 6 with no check?

henrikbrixandersen · 2025-06-22T16:01:26Z

dts/bindings/mtd/nvmem-consumer.yaml

+# Copyright (c) 2024, Andriy Gelman <[email protected]>
+# SPDX-License-Identifier: Apache-2.0
+
+include: base.yaml


I don't think this should include base.yaml as it does not depend on it.

henrikbrixandersen · 2025-06-22T16:02:37Z

include/zephyr/devicetree/nvmem.h

Ideally, we should have tests added for these macros.

nandojve · 2025-06-22T18:52:25Z

CC: @maass-hamburg

pdgendt · 2025-06-23T06:54:10Z

include/zephyr/devicetree/nvmem.h

+ *      eth: ethernet {
+ *		nvmem-cells = <&mac_address>;
+ *		nvmem-cell-names = "mac-address";
+ *	};
+ *
+ * Example usage:
+ *
+ *     DT_NVMEM_DEV_BY_IDX(DT_NODELABEL(eth), 0) // DEVICE_DT_GET(DT_NODELABEL(mac_eeprom))


Mixing tabs and spaces here

Suggested change

* eth: ethernet {

* nvmem-cells = <&mac_address>;

* nvmem-cell-names = "mac-address";

* };

*

* Example usage:

*

* DT_NVMEM_DEV_BY_IDX(DT_NODELABEL(eth), 0) // DEVICE_DT_GET(DT_NODELABEL(mac_eeprom))

* eth: ethernet {

* nvmem-cells = <&mac_address>;

* nvmem-cell-names = "mac-address";

* };

*

* Example usage:

*

* DT_NVMEM_DEV_BY_IDX(DT_NODELABEL(eth), 0) // DEVICE_DT_GET(DT_NODELABEL(mac_eeprom))

pdgendt · 2025-06-23T06:54:16Z

include/zephyr/devicetree/nvmem.h

+ *     eth: ethernet@ {
+ *		...
+ *		nvmem-cells = <&mac_address>;
+ *		nvmem-cell-names = "mac-address";
+ *		...
+ *     };
+ *
+ * Example usage:
+ *
+ *     DT_NVMEM_ADDR_BY_IDX(DT_NODELABEL(eth), 0) // 0xfa


Same here

Suggested change

* eth: ethernet@ {

* ...

* nvmem-cells = <&mac_address>;

* nvmem-cell-names = "mac-address";

* ...

* };

*

* Example usage:

*

* DT_NVMEM_ADDR_BY_IDX(DT_NODELABEL(eth), 0) // 0xfa

* eth: ethernet@ {

* ...

* nvmem-cells = <&mac_address>;

* nvmem-cell-names = "mac-address";

* ...

* };

*

* Example usage:

*

* DT_NVMEM_ADDR_BY_IDX(DT_NODELABEL(eth), 0) // 0xfa

pdgendt · 2025-06-23T06:56:37Z

drivers/ethernet/eth_xmc4xxx.c

 	return 0;
 }

+static int eth_xmc4xxx_get_mac_address_eeprom(const struct device *dev)


Instead of putting this in the drivers, wouldn't make sense to put this on a higher level and use eth_xmc4xxx_set_config to apply?

+1

maybe we can have some generic implementation for setting the mac address in
in net_if_post_init() just before bringing the iface up of subsys/net/ip/net_if.c.

What do you think? @jukkar @rlubos

should be after iface init, but before iface up

talih0 · 2025-07-07T05:43:53Z

I somehow missed the messages on this PR. Thank you to everyone for looking over. I'll send an updated version this week.

pdgendt · 2025-07-23T07:00:20Z

I somehow missed the messages on this PR. Thank you to everyone for looking over. I'll send an updated version this week.

@talih0 any update here?

maass-hamburg

should be implemented in a generic way, so that it uses the set_config api, so
that we don't have to duplicate code in every ethernet driver for that.

henrikbrixandersen · 2025-08-18T08:25:47Z

@talih0 Would you be open for pulling out the NVMEM provider/consumer commit of this PR and putting it its own PR? We have others requesting the same functionality for other use cases.

If you wish, I can pull it out and submit it with your approval.

GseoC · 2025-09-05T11:57:18Z

Hello,
We are also interested in the nvmem part. I'd like to use the nvmem-cells property to read the MAC address from fuses.
The thing is, with what is proposed here, this property is only used to get the phandle reference to the node. Therefore, it's the driver's job to call the appropriate API (EEPROM, OTP, ...). Wouldn't it be interesting to have a NVMEM abstraction layer that redirects the read/write operations to the appropriate driver using a provider/consumer mechanism (like the YAML teases it)? I'm new to Zephyr so do not hesitate to point-out any constraints that may challenge this idea.

zagor · 2025-09-26T07:51:54Z

I agree with others that this needs a more generic solution.

The mac address can be stored/obtained in a myriad different ways, only known by the application. Can we use an application-supplied callback that supplies the mac address? Then we don't have to implement every possible way addresses could be obtained.

pdgendt · 2025-09-26T08:11:06Z

I agree with others that this needs a more generic solution.

The mac address can be stored/obtained in a myriad different ways, only known by the application. Can we use an application-supplied callback that supplies the mac address? Then we don't have to implement every possible way addresses could be obtained.

I'll be working on a proof-of-concept using the NVMEM PR I opened this week.

zagor · 2025-09-26T08:57:58Z

The nvmem solution is nice, but I don't think it fully removes the need for a plain "use this mac address" approach.

The eeprom can be read protected or encrypted, or only store the last three bytes of the mac addres. The mac address could also be stored in a nand flash, or in a file on an emmc.

We can't solve every corner case in the device tree. We also need a way for the application code to simply provide the mac address.

maass-hamburg · 2025-09-26T09:02:42Z

The nvmem solution is nice, but I don't think it fully removes the need for a plain "use this mac address" approach.

The eeprom can be read protected or encrypted, or only store the last three bytes of the mac addres. The mac address could also be stored in a nand flash, or in a file on an emmc.

We can't solve every corner case in the device tree. We also need a way for the application code to simply provide the mac address.

There is already a api for that via set_config

zagor · 2025-09-26T09:32:05Z

There is already a api for that via set_config

Is there? I struggle to find how to do this early enough that no ethernet frames with the wrong mac address leak out of the interface.

pdgendt · 2025-09-26T09:44:07Z

The nvmem solution is nice, but I don't think it fully removes the need for a plain "use this mac address" approach.

The eeprom can be read protected or encrypted, or only store the last three bytes of the mac addres. The mac address could also be stored in a nand flash, or in a file on an emmc.

True, but it's a start.

There is already a api for that via set_config

Is there? I struggle to find how to do this early enough that no ethernet frames with the wrong mac address leak out of the interface.

Yeah, this is difficult indeed, for that reason I've created #96598 as an alternative here, but still do it in the driver init.

zephyrbot added platform: Infineon Infineon Technologies AG area: Ethernet area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jun 23, 2024

zephyrbot requested review from decsny, galak, ifyall, lmajewski, npal-cy and pdgendt June 23, 2024 14:54

zephyrbot assigned ifyall Jun 23, 2024

talih0 force-pushed the eth_eeprom branch from 1876d3d to 0d98034 Compare June 24, 2024 02:38

jukkar previously approved these changes Jun 24, 2024

View reviewed changes

pdgendt requested changes Jun 24, 2024

View reviewed changes

pdgendt previously requested changes Jun 24, 2024

View reviewed changes

github-actions bot added Stale and removed Stale labels Aug 25, 2024

henrikbrixandersen mentioned this pull request Aug 26, 2024

eth_nxp_enet : get mac address from eeprom #77525

Open

talih0 dismissed jukkar’s stale review via e9526c2 August 28, 2024 20:14

talih0 force-pushed the eth_eeprom branch from 0d98034 to e9526c2 Compare August 28, 2024 20:14

talih0 requested a review from nandojve as a code owner August 28, 2024 20:14

zephyrbot added area: Devicetree platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) labels Aug 28, 2024

zephyrbot requested review from attie-argentum, mnkp and stephanosio August 28, 2024 20:15

talih0 added 2 commits August 28, 2024 21:08

nandojve removed the Stale label Dec 18, 2024

nandojve approved these changes Dec 18, 2024

View reviewed changes

github-actions bot added the Stale label Apr 6, 2025

henrikbrixandersen removed the Stale label Apr 6, 2025

github-actions bot added the Stale label Jun 6, 2025

github-actions bot removed the Stale label Jun 10, 2025

henrikbrixandersen reviewed Jun 22, 2025

View reviewed changes

henrikbrixandersen requested changes Jun 22, 2025

View reviewed changes

pdgendt reviewed Jun 23, 2025

View reviewed changes

maass-hamburg requested changes Jul 23, 2025

View reviewed changes

kartben assigned sreeramIfx and unassigned ifyall Aug 28, 2025

pdgendt mentioned this pull request Sep 22, 2025

Add NVMEM support #96379

Merged

pdgendt mentioned this pull request Sep 26, 2025

[PoC] drivers: ethernet/wifi: Use NVMEM cell to get MAC address #96598

Draft

	Enable reading MAC address from external flash memory.
	Enable reading MAC address from external EEPROM memory.

drivers: eth_xmc4xxx: Add option to load MAC address from EEPROM #74835

Are you sure you want to change the base?

drivers: eth_xmc4xxx: Add option to load MAC address from EEPROM #74835

Conversation

talih0 commented Jun 23, 2024

Uh oh!

pdgendt left a comment

Choose a reason for hiding this comment

Uh oh!

jukkar commented Jun 24, 2024

Uh oh!

pdgendt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 25, 2024

Uh oh!

talih0 commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nandojve left a comment

Choose a reason for hiding this comment

Uh oh!

talih0 commented Feb 4, 2025

Uh oh!

github-actions bot commented Apr 6, 2025

Uh oh!

github-actions bot commented Jun 6, 2025

Uh oh!

talih0 commented Jun 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henrikbrixandersen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandojve commented Jun 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maass-hamburg Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talih0 commented Jul 7, 2025

Uh oh!

pdgendt commented Jul 23, 2025

Uh oh!

maass-hamburg left a comment

talih0 commented Aug 28, 2024 •

edited

Loading

maass-hamburg Jun 23, 2025 •

edited

Loading